[SOLVE]: constraint freedom visualisation #9573
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
Irev-Dev
changed the title
There was a problem hiding this comment.
I added these tests failing before I started working on a fix, now they pass.
| (ObjectId(1), Freedom::Fixed), // line1.start | ||
| (ObjectId(2), Freedom::Fixed), // line1.end | ||
| (ObjectId(4), Freedom::Fixed), // line2.start | ||
| (ObjectId(5), Freedom::Free), // line2.end (currently bug shows Conflict) | ||
| (ObjectId(7), Freedom::Conflict), // line3.start (currently bug shows Free) | ||
| (ObjectId(8), Freedom::Conflict), // line3.end (currently bug shows Free) |
There was a problem hiding this comment.
The kcl for this is
line1 with both ends fixed
line2 with one end fixed
line3 with two conflicting distance constraint
But before the fix, point_freedoms would come back as fixed, fixed, fixed, conflict, free, free.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 3
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Updated should_force_freedom_analysis to only return true if: The cache is empty (analysis hasn't run yet) AND all points are Free (likely default values) If the cache has values (even if all Free), it means analysis has already run, so we don't force another run. This prevents the repeated retries that led to incorrect results. What This Fixes Prevents the infinite loop of retries after each edit Stops forcing analysis when it has already run Reduces the chance of hitting the bug where freedom analysis returns incorrect results The underlying issue (freedom analysis sometimes returning 2 instead of 18 underconstrained) may still occur, but it will happen less often since we're not repeatedly retrying. Test this and see if segments still turn white. If the issue persists, we may need to investigate why kcl-ezpz freedom analysis returns inconsistent results with the same constraint counts.
We use the same logic in Rust to determine the freedom of a point since a point is made up of two vars, one for x and one for y. |
|
I put a number of things from your feedback into ff0b897 But I've realised there's another bug, It's too late here so will have to wait. |
| // Derive freedom from segment freedom | ||
| let freedomResult: Freedom | null = null | ||
| if (objects) { | ||
| const segmentObj = objects[idNum] |
There was a problem hiding this comment.
Object lookup uses ID as array index incorrectly
High Severity
The code uses objects[idNum] and objects[id] to look up segment objects, but this incorrectly treats the object ID as an array index. Object IDs are not guaranteed to match their position in the array (e.g., objects might have IDs [1, 2, 4, 5]). The correct approach is objects.find((o) => o?.id === id), which is already used correctly in deriveSegmentFreedom in segmentsUtils.ts. This bug causes freedom values to be looked up for the wrong objects or not found at all.
Additional Locations (1)
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| * 4. Conflict color (priority 4) - CONFLICT_COLOR (red) | ||
| * 5. Constrained color (priority 5) - TEXT_COLOR (white) |
There was a problem hiding this comment.
The comment incorrectly lists the priority order for Fixed and Free colors. The comment states:
- Priority 5: Constrained color (Fixed) - TEXT_COLOR (white)
- Priority 6: Unconstrained color (Free) - UNCONSTRAINED_COLOR (brand blue)
But the code implementation (lines 171-183) checks Free before Fixed, making Free priority 5 and Fixed priority 6. This matches the PR description's stated order ("free colour, fixed colour") but contradicts these comments.
Fix:
* 5. Unconstrained color (priority 5) - UNCONSTRAINED_COLOR (brand blue)
* 6. Constrained color (priority 6) - TEXT_COLOR (white)Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
jtran
left a comment
jtran
left a comment
There was a problem hiding this comment.
I commented a minor couple nits, and clippy is unhappy. But I think it's good otherwise.
Ostensibly trying to show colours in sketch mode,
Resolves #9281
but did a fair bit in rust:
On the TS side I revamp the colors a little to give clear prirotiy of what status wins out for colors, it's the following order now:
draft colour
hover colour
select colour
conflict colour
free colour
fixed colour
Also I added the following logic for deriving the freedom of segment bodies
Note
Strengthens freedom/conflict handling end-to-end and aligns UI coloring with solver outputs.
variables_in_conflicts; replaceunsatisfiedwithvariables_in_conflictsinSolved, addSolved::from_ezpz_outcomeand helpers to extract variable IDs; propagate conflict info to point freedom; default point freedom toFreewhen analysis absentPoint.freedomnon-null (Freedom), addpoint_freedom_cacheinFrontendStateto preserve previous freedoms when analysis didn’t run; updateupdate_state_after_execto merge/cache based onfreedom_analysis_ran; wire through calls (mock/engine, add/edit/delete) and add targeted tests for freedom analysis conflictssegmentsUtilswithderiveSegmentFreedomandgetSegmentColor; refactor segment rendering to pass/storefreedom, compute hover/selection/draft colors by precedence (draft > hover > select > conflict > free > fixed); compute segment freedom from endpoint points; add comprehensive unit tests for freedom derivation and color selectionWritten by Cursor Bugbot for commit 90eb426. This will update automatically on new commits. Configure here.